Sponge state remapping #755

Al-Kindi-0 · 2026-01-07T11:47:54Z

Describe your changes

Addresses #673

Checklist before requesting a review

Repo forked and branch created from next according to naming convention.
Commit messages and codestyle follow conventions.
Relevant issues are linked in the PR description.
Tests added for new functionality.
Documentation/comments updated according to changes.

bobbinth · 2026-01-08T06:20:36Z

miden-crypto/src/merkle/smt/full/mod.rs

    fn key_to_leaf_index(key: &Word) -> LeafIndex<SMT_DEPTH> {
-        let most_significant_felt = key[3];
-        LeafIndex::new_max_depth(most_significant_felt.as_canonical_u64())
+        let least_significant_felt = key[0];
+        LeafIndex::new_max_depth(least_significant_felt.as_canonical_u64())
    }


I'm not 100% sure we should change this. Conceptually, if we interpret the word as a 256-bit integer, the most significant bits are the ones that define the path of the first 64 levels of the tree. So, it may still make sense to keep the most significant element define the leaf index.

What's the main motivation for changing this? Is it to avoid doing dup.3 when trying to get the index for SMTs in MASM?

I see, so in our trees we are encoding the paths from MSB to LSB and hence, as you suggested, key[3] is the right choice and hence this should be reverted.

Though, I would claim that MSB to LSB is not intuitive and the LSB to MSB makes more sense, any reasons for picking one over the other ?

I think this is mostly an extension of thinking of leaf indexes as integers. For example, if we have a tree of depth 64, the first leaf would be at index 0, the second one at index 1, and the last at index u64::MAX. This naturally means that the most significant bit specifies if the leaf is in the left or right subtree immediately under the root, and the next most significant bit specifies the next subtree etc.

If we extended this to a tree of depth 256, each leaf position would be encoded as a 256-bit integer. And then, again, the most significant would define left/right subtree under the root etc.

Since we assume that the layout of 256-bit integers would be in little-endian form, then most significant bits would be located in key[3]. We could, of course, keep them in key[0] but then:

There is a slight inconsistency in where the relevant bits are. For example, if we write the key as bytes (in little endian form), i.e., 32 bytes - the byte with most significant bits would be at key_bytes[7] which is somewhat counterintuitive.

Contrast this with key[3] - here, most significant bits would be in key_bytes[31]. Which I think is a bit more consistent.

Also, keeping things as is (i.e., using key[3]) should probably result in fewer changes, right?

I agree, there are only two consistent choices here, either MSB or LSB throughout the 256-bit integer (including within the individual limbs).

To me LSB to MSB is the most intuitive as then we get

the first limb K[0] is the most accessible on the op stack

bit 0 decides the root's children, while bit 255 decides the leaves of the tree.

Of course, these will imply a number of changes but nothing major as most of these have already been done.

In any case, the changes are reverted for now and we can come back to this question in the future.

My 2¢: I agree that it's generally more intuitive to order the leaves in "natural" order (i.e. MSB to LSB), though I've often encountered situations where algorithms can be a bit faster if we use "bit-reversed" order. However, the latter is less intuitive and led me down the wrong path a few times. I don't have a strong opinion here, but we could discuss this in a separate issue.

Agreed, though for the VM I would always err on the side of consistency and readability provided the cost for doing so is manageable.
In my mind, this is the last remaining point that is user-facing in order to bring a coherent orientation to the VM, I believe.

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/mod.rs

miden-crypto/src/hash/algebraic_sponge/poseidon2/mod.rs

bobbinth · 2026-01-10T00:13:32Z

miden-crypto/src/hash/algebraic_sponge/mod.rs

+/// The first and second 4-element words of the rate portion.
+pub(crate) const INPUT1_RANGE: Range<usize> = 0..4;
+pub(crate) const INPUT2_RANGE: Range<usize> = 4..8;


I think it may be worth exposing these ranges publicly. I'll make a small commit to do this.

bobbinth

Looks great! Thank you! I left a couple of comment-related comments inline.

miden-crypto/src/dsa/falcon512_rpo/tests/data.rs

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/tests.rs

adr1anh

Looks good to me!

CHANGELOG.md

adr1anh · 2026-01-13T11:32:39Z

miden-crypto/src/merkle/smt/full/mod.rs

    fn key_to_leaf_index(key: &Word) -> LeafIndex<SMT_DEPTH> {
-        let most_significant_felt = key[3];
-        LeafIndex::new_max_depth(most_significant_felt.as_canonical_u64())
+        let least_significant_felt = key[0];
+        LeafIndex::new_max_depth(least_significant_felt.as_canonical_u64())
    }


My 2¢: I agree that it's generally more intuitive to order the leaves in "natural" order (i.e. MSB to LSB), though I've often encountered situations where algorithms can be a bit faster if we use "bit-reversed" order. However, the latter is less intuitive and led me down the wrong path a few times. I don't have a strong opinion here, but we could discuss this in a separate issue.

plafer

LGTM

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/tests.rs

plafer · 2026-01-13T21:57:24Z

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/mod.rs

+    /// The first 4-element word of the rate portion.
+    pub const INPUT1_RANGE: Range<usize> = INPUT1_RANGE;
+
+    /// The second 4-element word of the rate portion.
+    pub const INPUT2_RANGE: Range<usize> = INPUT2_RANGE;


We use "rate" everywhere else, why do we use "input" here? Wouldn't it be clearer/more consistent to use RATE0_RANGE and RATE1_RANGE?

e.g. we have STATE_RATE_0_RANGE and STATE_RATE_1_RANGE in the VM

I think your point is valid, but the use of input is sometimes useful when for example we have a left child and a right child of a Merkle tree node. We can then say that the left child is input1 and the right child is input2.

Went ahead and changed toRATE0 and RATE1 to stay consistent both here and in the VM.

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/mod.rs

bobbinth reviewed Jan 8, 2026

View reviewed changes

Al-Kindi-0 force-pushed the al-sponge-state-remapping-v2 branch from 5571ba1 to 8dfedbe Compare January 8, 2026 07:38

huitseeker reviewed Jan 8, 2026

View reviewed changes

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/mod.rs Show resolved Hide resolved

miden-crypto/src/hash/algebraic_sponge/poseidon2/mod.rs Show resolved Hide resolved

Al-Kindi-0 force-pushed the al-sponge-state-remapping-v2 branch from 56ef32e to 6eed157 Compare January 8, 2026 09:56

Al-Kindi-0 mentioned this pull request Jan 9, 2026

Change stack ordering through unified LE convention and sponge state remapping 0xMiden/miden-vm#2547

Merged

Al-Kindi-0 force-pushed the al-sponge-state-remapping-v2 branch 2 times, most recently from 7907951 to 5cc55be Compare January 9, 2026 10:17

bobbinth reviewed Jan 10, 2026

View reviewed changes

bobbinth approved these changes Jan 10, 2026

View reviewed changes

miden-crypto/src/dsa/falcon512_rpo/tests/data.rs Outdated Show resolved Hide resolved

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/tests.rs Show resolved Hide resolved

Al-Kindi-0 mentioned this pull request Jan 10, 2026

Generate the expected hash outputs of RPO using reference implementation #768

Open

adr1anh self-requested a review January 12, 2026 07:25

adr1anh approved these changes Jan 13, 2026

View reviewed changes

plafer approved these changes Jan 13, 2026

View reviewed changes

huitseeker approved these changes Jan 14, 2026

View reviewed changes

miden-crypto/src/hash/algebraic_sponge/rescue/rpo/mod.rs Outdated Show resolved Hide resolved

Al-Kindi-0 and others added 13 commits January 14, 2026 13:35

chore: changed the layout of the sponge state

e38f00d

fix fmt

2f2675a

feat: update word comparison to LE convention

cdc2741

fix comments

693f8e8

undo changes to word order

3f18eb4

fix stale comments

4e66555

remap digest to be top word of state

eb1df14

fix lint

2aa54d9

chore: expose INPUT1_RANGE and INPUT2_RANGE publicly

6d17c2b

update comment deterministic signature Falcon

84af1c7

chore: move input range constants into the correct struct

97cdd07

update comments digest position

1544ed1

address feedback

1d739ec

huitseeker force-pushed the al-sponge-state-remapping-v2 branch from 4c62719 to 1d739ec Compare January 14, 2026 18:50

huitseeker merged commit b2122b7 into next Jan 14, 2026
27 checks passed

huitseeker deleted the al-sponge-state-remapping-v2 branch January 14, 2026 19:05

huitseeker mentioned this pull request Jan 14, 2026

Change the layout of the sponge state #707

Closed

Sponge state remapping #755

Sponge state remapping #755

Uh oh!

Conversation

Al-Kindi-0 commented Jan 7, 2026

Describe your changes

Checklist before requesting a review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adr1anh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plafer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants